-
Notifications
You must be signed in to change notification settings - Fork 70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Django 4.2 support #334
Conversation
Thanks for the pull request, @salman2013! Please note that it may take us up to several weeks or months to complete a review and merge your PR. Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. Please let us know once your PR is ready for our review and all tests are green. Once you've signed the CLA, please allow 1 business day for it to be processed. After this time, you can re-run the CLA check by editing the PR title. If the problem persists, you can tag the |
1 similar comment
Thanks for the pull request, @salman2013! Please note that it may take us up to several weeks or months to complete a review and merge your PR. Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. Please let us know once your PR is ready for our review and all tests are green. Once you've signed the CLA, please allow 1 business day for it to be processed. After this time, you can re-run the CLA check by editing the PR title. If the problem persists, you can tag the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@salman2013, I just merged dependency updates. Please rebase on the current master
branch.
.github/workflows/ci.yml
Outdated
@@ -28,7 +28,7 @@ jobs: | |||
matrix: | |||
os: [ubuntu-20.04] | |||
python-version: [3.8] | |||
toxenv: [py38, integration, quality, translations] | |||
toxenv: [py38, django32, django42, integration, quality, translations] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
py38
is redundant. It would make sense to use both Django 3.2 and 4.2 for integration
, quality
, and translations
, though.
Hi @salman2013, thank you for this contribution! It looks like you haven't submitted a signed Contributor Agreement yet. To do that you can follow the instructions from the PR bot's latest post above. If you have any questions, let us know. |
d0acfbb
to
f746b1f
Compare
.github/workflows/ci.yml
Outdated
@@ -28,7 +28,7 @@ jobs: | |||
matrix: | |||
os: [ubuntu-20.04] | |||
python-version: [3.8] | |||
toxenv: [py38, integration, quality, translations] | |||
toxenv: [django32, django42, integration-django32, integration-django42, quality-django32, quality-django42, translations-django32, translations-django42] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All these environments are running unit tests. See the CI logs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking into it.
@itsjeyd My PR is not an open source contribution, I have verified CLA from Axim. |
@salman2013 All pull requests issued against the Signing the CLA has nothing to do with that. It's a prerequisite for everyone who wants to get changes reviewed and merged into the Open edX code base. I hope that helps, and if you have any additional questions just let me know. |
Hi @itsjeyd I believe there is some confusion here. @salman2013 is a new member in the |
# Conflicts: # requirements/quality.txt # requirements/test.txt # requirements/workbench.txt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
- I tested this: checked that the CI is passing
- I read through the code
- I checked for accessibility issues: n/a
- Includes documentation: n/a
- I made sure any change in configuration variables is reflected in the corresponding client's
configuration-secure
repository: n/a
OK I see, thanks for clarifying! I removed the |
@Agrendalath Could you please bump the version and release this update so that we can close the ticket? We can also do it, Please confirm, thanks |
@salman2013, why should we release it? This only changes the tests - we didn't change any code or base requirements here, so the distributed Python package will be exactly the same as the previous one. |
Hi @Agrendalath |
@UsamaSadiq, I'd like to understand this use case a bit more to be sure how the changes in the specific XBlock repositories affect things down the line. The
As you can see in the setup.py, the only requirements used while installing this Python package are in the base.in (with its constraints defined in constraints.txt). Furthermore, you can check which files are included in the package (pushed to the PyPI) by checking these lines in the setup.py + the MANIFEST.in file. You can verify this by running .
├── Changelog.md
├── drag_and_drop_v2
│ ├── __init__.py
│ ├── compat.py
│ ├── default_data.py
│ ├── drag_and_drop_v2.py
│ ├── public
│ │ ├── css
│ │ │ ├── drag_and_drop.css
│ │ │ └── drag_and_drop_edit.css
│ │ ├── img
│ │ │ └── triangle.png
│ │ ├── js
│ │ │ ├── drag_and_drop.js
│ │ │ ├── drag_and_drop_edit.js
│ │ │ ├── translations
│ │ │ │ ├── ar
│ │ │ │ │ └── text.js
│ │ │ │ ├── de
│ │ │ │ │ └── text.js
│ │ │ │ ├── en
│ │ │ │ │ └── text.js
│ │ │ │ ├── eo
│ │ │ │ │ └── text.js
│ │ │ │ ├── es_419
│ │ │ │ │ └── text.js
│ │ │ │ ├── fr
│ │ │ │ │ └── text.js
│ │ │ │ ├── he
│ │ │ │ │ └── text.js
│ │ │ │ ├── hi
│ │ │ │ │ └── text.js
│ │ │ │ ├── it
│ │ │ │ │ └── text.js
│ │ │ │ ├── ja
│ │ │ │ │ └── text.js
│ │ │ │ ├── ko
│ │ │ │ │ └── text.js
│ │ │ │ ├── nl
│ │ │ │ │ └── text.js
│ │ │ │ ├── pl
│ │ │ │ │ └── text.js
│ │ │ │ ├── pt_BR
│ │ │ │ │ └── text.js
│ │ │ │ ├── pt_PT
│ │ │ │ │ └── text.js
│ │ │ │ ├── ru
│ │ │ │ │ └── text.js
│ │ │ │ ├── tr
│ │ │ │ │ └── text.js
│ │ │ │ └── zh_CN
│ │ │ │ └── text.js
│ │ │ └── vendor
│ │ │ ├── handlebars-v1.1.2.js
│ │ │ └── virtual-dom-1.3.0.min.js
│ │ └── themes
│ │ └── apros.css
│ ├── templates
│ │ └── html
│ │ ├── drag_and_drop.html
│ │ ├── drag_and_drop_edit.html
│ │ └── js_templates.html
│ ├── translations
│ │ ├── __init__.py
│ │ ├── ar
│ │ │ └── LC_MESSAGES
│ │ │ ├── text.mo
│ │ │ └── text.po
│ │ ├── config.yaml
│ │ ├── de
│ │ │ └── LC_MESSAGES
│ │ │ ├── text.mo
│ │ │ └── text.po
│ │ ├── en
│ │ │ └── LC_MESSAGES
│ │ │ ├── django.po
│ │ │ ├── text.mo
│ │ │ └── text.po
│ │ ├── eo
│ │ │ └── LC_MESSAGES
│ │ │ ├── text.mo
│ │ │ └── text.po
│ │ ├── es_419
│ │ │ └── LC_MESSAGES
│ │ │ ├── text.mo
│ │ │ └── text.po
│ │ ├── fr
│ │ │ └── LC_MESSAGES
│ │ │ ├── text.mo
│ │ │ └── text.po
│ │ ├── fr_CA
│ │ │ └── LC_MESSAGES
│ │ │ ├── text.mo
│ │ │ └── text.po
│ │ ├── he
│ │ │ └── LC_MESSAGES
│ │ │ ├── text.mo
│ │ │ └── text.po
│ │ ├── hi
│ │ │ └── LC_MESSAGES
│ │ │ ├── text.mo
│ │ │ └── text.po
│ │ ├── it
│ │ │ └── LC_MESSAGES
│ │ │ ├── text.mo
│ │ │ └── text.po
│ │ ├── ja
│ │ │ └── LC_MESSAGES
│ │ │ ├── text.mo
│ │ │ └── text.po
│ │ ├── ko
│ │ │ └── LC_MESSAGES
│ │ │ ├── text.mo
│ │ │ └── text.po
│ │ ├── nl
│ │ │ └── LC_MESSAGES
│ │ │ ├── text.mo
│ │ │ └── text.po
│ │ ├── pl
│ │ │ └── LC_MESSAGES
│ │ │ ├── text.mo
│ │ │ └── text.po
│ │ ├── pt_BR
│ │ │ └── LC_MESSAGES
│ │ │ ├── text.mo
│ │ │ └── text.po
│ │ ├── pt_PT
│ │ │ └── LC_MESSAGES
│ │ │ ├── text.mo
│ │ │ └── text.po
│ │ ├── rtl
│ │ │ └── LC_MESSAGES
│ │ │ ├── text.mo
│ │ │ └── text.po
│ │ ├── ru
│ │ │ └── LC_MESSAGES
│ │ │ ├── text.mo
│ │ │ └── text.po
│ │ ├── settings.py
│ │ ├── tr
│ │ │ └── LC_MESSAGES
│ │ │ ├── text.mo
│ │ │ └── text.po
│ │ └── zh_CN
│ │ └── LC_MESSAGES
│ │ ├── text.mo
│ │ └── text.po
│ └── utils.py
├── LICENSE
├── MANIFEST.in
├── PKG-INFO
├── README.md
├── requirements
│ ├── base.in
│ └── constraints.txt
├── setup.cfg
├── setup.py
└── xblock_drag_and_drop_v2.egg-info
├── dependency_links.txt
├── entry_points.txt
├── PKG-INFO
├── requires.txt
├── SOURCES.txt
└── top_level.txt None of the files updated in this PR are included in the package pushed to PyPI. I'm not against releasing a new version, but I'm curious how releasing the package affects your testing process.
As we do not use anything from this PR outside of tests, and the CI is passing, these changes cannot break anything. |
Description:
Codemod used to upgrade this repo: https://github.com/adamchainz/django-upgrade
Ticket: edx/upgrades#183